Fix template path validator for composable integration packages#1108
Fix template path validator for composable integration packages#1108teresaromero wants to merge 2 commits intoelastic:mainfrom
Conversation
…dd example test packages When a policy template input uses `package` instead of `type`, or a data stream stream uses `package` instead of `input`, the Go zero value for the missing string field is `""`. The existing template path validator matched these empty strings against each other, then tried to open a non-existent `agent/stream` directory, causing a false validation failure on any composable integration package. Fix by skipping template path validation for inputs and streams that carry a `package` reference, since configuration is inherited from the referenced input package and no local agent stream template exists. Add three example test packages covering all composable cases from the spec: - good_composable_pt_inputs: all policy template inputs use package reference - good_composable_ds_streams: all data stream streams use package reference - good_composable_multi_package: multiple input packages in both locations, plus requires.content dependencies Made-with: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThis PR introduces composable package support to the validator. A new Package field is added to policy template inputs and data streams. The validation logic in the semantic validator is updated to skip local template_path validation checks for inputs and streams that reference a package. Three new test packages demonstrate composable scenarios: one with policy template inputs referencing packages, one with data stream streams referencing packages, and one using multiple input packages across different data streams. Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
💚 Build Succeeded
|
| "good_composable_pt_inputs": {}, | ||
| "good_composable_ds_streams": {}, |
There was a problem hiding this comment.
Nit.
| "good_composable_pt_inputs": {}, | |
| "good_composable_ds_streams": {}, | |
| "good_composable_policy_template_inputs": {}, | |
| "good_composable_data_stream_streams": {}, |
| link: https://github.com/elastic/package-spec/pull/1092 | ||
| - description: Fix template path validator to skip policy template inputs and data stream streams that use package references. | ||
| type: bugfix | ||
| link: https://github.com/elastic/package-spec/pull/1108 |
There was a problem hiding this comment.
No need to add changelog for this as package references are also introduced in 3.6.
| "good_composable_pt_inputs": {}, | ||
| "good_composable_ds_streams": {}, | ||
| "good_composable_multi_package": {}, |
There was a problem hiding this comment.
Could these cases be added to existing good_packages instead of adding a test package for each one?
There was a problem hiding this comment.
Btw, good_composable_pt_inputs and good_composable_ds_streams also pass with the code in main. Is this expected?
| title: Web server logs | ||
| type: logs | ||
| streams: | ||
| - package: filelog_otel |
There was a problem hiding this comment.
Is this package correct in a real case? This input is not used on any policy template 🤔 Only logfile is used.
There was a problem hiding this comment.
i dont know if is a real-case, but is a possible case...
when the policy template has certain imputs and the required package is at a data stream, there is no "type: logfile" linking policy template with streams. is this possible?
| func validateIntegrationPackagePolicyTemplate(fsys fspath.FS, policyTemplate integrationPolicyTemplate, dsManifestMap map[string]dataStreamManifest) error { | ||
| for _, input := range policyTemplate.Inputs { | ||
| // inputs using package references inherit configuration from the input package, | ||
| // so there is no local agent stream template to validate. |
There was a problem hiding this comment.
This will be still validated on the built package, right?
There was a problem hiding this comment.
i dont understand the question :S
Before building, this validation takes place. Here we are looking for the template_path so we check if exist. Data streams and policy templates are "linked" by the input.type -> stream.input;
when using package, the validation does not find template path, and injects empty "type" into the stream validation, to eventually find the template path.. when in the stream template validation, it finds that package is used, so input is also ""; empty == empty; it looks for the default stream.yml.hbs and it does not find it.
Shall this "connection" be fixed? perhaps i took a wrong assumption
There was a problem hiding this comment.
I think the code is fine, I was only asking for confirmation.
elastic-package build validates the built package after building it. In this case built packages will have streams here even if their source code didn't have them.
There was a problem hiding this comment.
Umm, actually I see that streams merge is not described on any issue, apart from variables and template paths. I will create one.
| title: Database metrics | ||
| type: metrics | ||
| streams: | ||
| - input: logfile |
There was a problem hiding this comment.
This input is not used on any policy template. Would this work as a real package?
There was a problem hiding this comment.
same as the other example, out of the spec this is possible for a package, i created them based on what possibilities a package can have. is there a requirement for data streams and inputs from policy templates to be "conected". i remember the conexion is between the input type (policy) and the input (Stream). they should appear allways? i have a gap here.
if a policy template has a package, with a template, does it make sense to have a data stream with templates too?
What does this PR do?
Fixes the
ValidateIntegrationPolicyTemplatessemantic validator to correctly handle integration packages that use the 3.6.0 composability feature (packagereferences in policy template inputs and data stream streams).Adds three example test packages that cover all possible composable cases defined in the spec.
Why is it important?
While working on enabling composable integrations in elastic-package, validation was failing on any integration package that used
package:references in policy template inputs or data stream streams — even when the package was otherwise valid.The root cause is a zero-value collision in the Go structs. When a policy template input uses
packageinstead oftype, theTypefield unmarshals to"". Similarly, when a data stream stream usespackageinstead ofinput, theInputfield is also"". The validator matched these two empty strings, then tried to opendata_stream/<name>/agent/stream— a directory that doesn't exist for composable streams (they inherit all configuration from the referenced input package), producing a falseno such file or directoryerror.The fix adds a
Packagefield to both structs and skips template path validation whenever apackagereference is present.Checklist
test/packagesthat prove my change is effective.spec/changelog.yml.Related issues
Made with Cursor